You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the case of ML-KEM and ML-DSA keys there are no algorithm parameters. So, this code adds a NULL to the encoding when it shouldn't. The issue in jdk17 is when you are trying to compare keys to make sure that they are equal between providers. However, the equals method makes use of the encoding to do the compare which will fail.
The fix for this was already there in JDK 21 and later.
The backport could not be done as a clean cherry-pick because I had manually remove the old logic that adds a NULL when the OID isn't in the list, and replaced it with equivalent behavior that only adds a NULL when required.
Progress
Change must be properly reviewed (1 review required, with at least 1 Reviewer)
👋 Welcome back varadam! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.
openjdkbot
changed the title
Backport a97271e3b5d5a08fc503a11cd3e253974fb77ce6
8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded
Jun 2, 2025
/approval 8301793 request the backport fix where the AlgorithmId should omit the parameters field when it is not required, rather than encoding it as NULL, unless the algorithm explicitly mandates. JTREG testing successful for sun/security
Hi Varada,
please first get a review, then label for approval.
Please edit the description of the PR and tell why this is not clean.
I'm not sure this is a good backport. Please tell in the approval comment in JBS why you
want to backport this and give a risk assessment.
⚠️@varada1110 This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.
Thank you @wangweij for the review
/approval 8301793 request the backport fix where the AlgorithmId should omit the parameters field when it is not required, rather than encoding it as NULL, unless the algorithm explicitly mandates. JTREG testing successful for sun/security
Hi @varada1110
please state in the PR comment why this is not clean.
please give a reason why you want this in 17 (besides telling this fixes the error it is supposed to fix...).
GHA and the related tests are a minimum. Did you do any other testing?
(I succesfully ran this through SAPs testing)
Hi @varada1110 please state in the PR comment why this is not clean. please give a reason why you want this in 17 (besides telling this fixes the error it is supposed to fix...). GHA and the related tests are a minimum. Did you do any other testing? (I succesfully ran this through SAPs testing)
Hi @GoeLin ,
I have updated the PR description and added why PR Is not clean and reason for backporting it to jdk17.
Thank you for testing from your end. I have only done jtreg testing. Do I need to perform any other testing, please let me know.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
openjdk
bot
changed the title
In the case of ML-KEM and ML-DSA keys there are no algorithm parameters. So, this code adds a NULL to the encoding when it shouldn't. The issue in jdk17 is when you are trying to compare keys to make sure that they are equal between providers. However, the equals method makes use of the encoding to do the compare which will fail.
The fix for this was already there in JDK 21 and later.
The backport could not be done as a clean cherry-pick because I had manually remove the old logic that adds a NULL when the OID isn't in the list, and replaced it with equivalent behavior that only adds a NULL when required.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/3615/head:pull/3615$ git checkout pull/3615Update a local copy of the PR:
$ git checkout pull/3615$ git pull https://git.openjdk.org/jdk17u-dev.git pull/3615/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 3615View PR using the GUI difftool:
$ git pr show -t 3615Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/3615.diff
Using Webrev
Link to Webrev Comment